-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
record memory and op supplement info #43550
record memory and op supplement info #43550
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
… record_memory_and_op_supplement_info_for_profiler
@@ -14,6 +14,8 @@ | |||
|
|||
#pragma once | |||
|
|||
#include <map> | |||
#include <stack> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
删掉吧
@@ -26,6 +27,9 @@ void CPUPinnedAllocator::FreeImpl(phi::Allocation *allocation) { | |||
PADDLE_ENFORCE_GPU_SUCCESS(cudaFreeHost(allocation->ptr())); | |||
#endif | |||
HOST_MEMORY_STAT_UPDATE(Reserved, 0, -allocation->size()); | |||
platform::RecordMemEvent(allocation->ptr(), allocation->place(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should pass CPUPlace here? Better to handle GPUPinnedPlace in the implement of RecordMemEvent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -54,6 +57,9 @@ class StatAllocator : public Allocator { | |||
DEVICE_MEMORY_STAT_UPDATE(Allocated, place.GetDeviceId(), | |||
allocation->size()); | |||
} | |||
platform::RecordMemEvent(allocation->ptr(), allocation->place(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same problem as above to handle GPUPinnedPlace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
RecordMemEvent::RecordMemEvent(const void *ptr, const phi::Place &place, | ||
size_t size, const TracerMemEventType type) { | ||
if (g_state == ProfilerState::kDisabled && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to keep consistent of the curly braces requirement in if statement. Since elsewhere you add a curly brace for if even though the statement is brief, better to add here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
This reverts commit c1d4df5.
… record_memory_and_op_supplement_info_for_profiler
… record_memory_and_op_supplement_info_for_profiler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* record memory and op supplement info * update * update * fix a bug * fix memory recording * fix a bug * update * update * fix a bug * update * fix a bug * fix a bug * fix a bug * Revert "fix a bug" This reverts commit c1d4df5. * fix a bug * fix format * fix
* record memory and op supplement info * update * update * fix a bug * fix memory recording * fix a bug * update * update * fix a bug * update * fix a bug * fix a bug * fix a bug * Revert "fix a bug" This reverts commit c1d4df5. * fix a bug * fix format * fix
PR types
Others
PR changes
Others
Describe